-
Notifications
You must be signed in to change notification settings - Fork 174
Было сложно, но я сделал #71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
antonkazakov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Привет. Оставил замечания
|
|
||
| override fun onCreate() { | ||
| super.onCreate() | ||
| appComponent = DaggerApplicationComponent.factory().create(this, DataModule()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А зачем ты модуль прокинул в фабрику?
| @BindsInstance | ||
| @Named("ApplicationContext") | ||
| appContext: Context, | ||
| dataModule: DataModule, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это нужно убрать
|
|
||
| @Named("ApplicationContext") | ||
| fun provideContext(): Context | ||
| fun provideColorFlow(): MutableSharedFlow<Int> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нужно их перенести на уровень активити компонента
|
|
||
|
|
||
| @Module | ||
| @Named("DataModule") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А это зачем?)
|
|
||
| @Module | ||
| @Named("DataModule") | ||
| class DataModule { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Подключи его в активитикомпонент
| import javax.inject.Named | ||
|
|
||
| class ViewModelProducer @Inject constructor( | ||
| @Named("ActivityContext") private val context: Context, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
У тебя здесь будет утечка памяти, потому что ссылка на активити контекс будет жить в ВМ, а активити будет пересоздана
| @Named("ActivityContext") private val context: Context, | ||
| private val colorGenerator: ColorGenerator, | ||
| private val colorFlow: MutableSharedFlow<Int>, | ||
| private val stateFlow: MutableSharedFlow<State> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не совсем понял зачем тебе 2 флоу
| private val colorFlow: MutableSharedFlow<Int>, | ||
| private val stateFlow: MutableSharedFlow<State> | ||
| ) : ViewModel(){ | ||
| val state = MutableLiveData<State>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Правильнее конечно чтобы публичная лайвдата была иммутабельная
|
|
||
| override fun onViewCreated(view: View, savedInstanceState: Bundle?) { | ||
| super.onViewCreated(view, savedInstanceState) | ||
| (requireActivity() as MainActivity).mainActivityComponent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я бы это перенес в oncreate/onattach, onViewCreated слишком часто вызывается
|
|
||
| class ViewModelReceiver @Inject constructor( | ||
| @Named("ApplicationContext") private val context: Context, | ||
| private val colorFlow: MutableSharedFlow<Int>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Неправильно что у тебя в приемнике есть возможность эмитить эвенты во флоу, здесь должен быть SharedFlow
Ради интереса еще сделал обратный отклик из Ресивер фрагмента в Продюсер фрагмент.
Но тема даггера все еще сложновата =( Буду пересматривать лекции